-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure Each TestQueueRow Uses a Unique ID-ref. #370
Conversation
@jkva I understand the intent behind this, but I'm not sure I understand the implementation/resulting mark-up. Each row in the test queue represents a test plan that can be assigned to multiple people, not just one. |
@jscholes previously, you would get multiple elements with id |
@jkva I looked at the rendered mark-up in the actual app as it is currently deployed, and this makes more sense now. The use of the word "row" is what was tripping me up. You're talking about a However, now that I understand it, I want to confirm that the proposed change fully resolves the issue. The Test Queue will contain multiple instances of the same plan, across different browser/AT combinations. If the same tester is assigned to the same plan across multiple combos, will the IDs still be unique? |
That's a good use case for me to double-check, thank you. |
@jscholes I've pushed a commit including the Test Plan Run ID within the generated ID, further guaranteeing uniqueness. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the optional change request.
currentUserTestPlanRun.id, | ||
'assignee', | ||
tester.username, | ||
'completed' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional nitpick to address: I understand the id's original structure was being preserved with -completed
being appended. Taking another look again at its use, perhaps something like -completed-tests
is clearer for the id's purpose, but removing -completed
altogether also works well and is helpful to reduce some of the id's verbosity.
This change ensures each id generated for a Test Queue Row combines its test plan ID and tester name. This addresses a problem where two test rows assigned to the same tester generate (elements with) duplicate IDs.